fix(discovery): sort discovered addresses by connectability - #37
Merged
Conversation
`mdns_sd::ServiceInfo::get_addresses` returns a `&HashSet<IpAddr>`, and all
three browsers collected it straight into a `Vec` with
`.iter().copied().collect()`. Callers then dial `addresses.first()`, so the
address actually connected to was an arbitrary hash-set element — not merely
unsorted but non-deterministic, and re-rolled every time a record refreshed.
Observed across a single run, same machine, same service:
[192.168.0.97, fe80::1427:c2c8:a235:a115]
[fe80::1, ::1, fe80::1427:c2c8:a235:a115, 127.0.0.1, 192.168.0.97]
[127.0.0.1, ::1, fe80::1, 192.168.0.97, fe80::1427:c2c8:a235:a115]
[::1, fe80::1, 127.0.0.1]
So `first()` yielded 192.168.0.97, fe80::1, 127.0.0.1 or ::1 depending on
timing, and the fourth record offered no routable address at all. Remote
receivers only advertise two addresses so they usually looked fine, which is
what kept this hidden; a machine advertising loopback and link-local
alongside its LAN address exposes it immediately.
Sort at collection instead, most-connectable first: routable IPv4, routable
IPv6, loopback, link-local, unspecified — ties broken on the address value so
the result is stable run to run. Link-local ranks below loopback because we
carry no zone index and cannot reconstruct one, so `fe80::1` is not dialable
at all, whereas loopback at least resolves (to the wrong host for a remote
receiver, hence still below anything routable). IPv4 is preferred over
routable IPv6 since it needs no scope handling.
This makes `addresses.first()` correct by construction, so the three `addr()`
arms in the sender need no change.
Verified against the live network: every discovered receiver now reports a
routable IPv4 first, on every refresh.
snadahalli
force-pushed
the
fix/receiver-address-selection
branch
from
August 26, 2026 09:38
711ee74 to
55a1f15
Compare
Developer1010x
pushed a commit
that referenced
this pull request
Aug 26, 2026
…43) #42 was merged twelve seconds after #41, into `fix/airplay-hkp-header` — which #41 had just merged into master and left behind. The merge succeeded, so nothing looked wrong, but the commits landed on a branch nothing points at and master never received them. Master therefore has the `X-Apple-HKP` header from #41 and none of what it was a prerequisite for: the SRP proof still hashes g padded, transient pairing still runs M5/M6 and gets the connection closed, and there is no encrypted control channel. Pairing is broken on master in exactly the way #42 fixed. This restores #42's own diff — the eight `openplay-airplay` files it actually touched — on top of current master. Deliberately *not* a merge of `fix/airplay-hkp-header`. That branch was cut before #35, #36, #37, #39 and #40 merged, so a diff against it reads as deleting `openplay-discovery/src/address.rs` and reverting 244 lines of `openplay-sender/src/app.rs`. Merging it would silently undo five landed fixes. Only the range between #42 and its own parent is safe to replay, and that range touches nothing outside `openplay-airplay`. Verified after the replay: 233 tests pass, clippy and fmt clean, and the work from #35/#36/#37/#40 is still in the tree. Co-authored-by: Sandeepa Nadahalli <1698507+snadahalli@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
mdns_sd::ServiceInfo::get_addressesreturns a&HashSet<IpAddr>, and all three browsers collected it straight into aVec:Callers then dial
addresses.first(). So the address actually connected to was an arbitrary hash-set element — not merely unsorted but non-deterministic, and re-rolled every time a record refreshed.Observed across a single run, same machine, same service:
first()yielded192.168.0.97,fe80::1,127.0.0.1or::1depending on timing, and the fourth record offered no routable address at all. Remote receivers advertise only two addresses so they usually looked fine, which is what kept this hidden — a machine advertising loopback and link-local alongside its LAN address exposes it immediately.The fix
Sort at collection, most-connectable first:
Ties break on the address value, so the result is stable run to run in a way
HashSetiteration order is not. Link-local ranks below loopback deliberately:fe80::1without a zone cannot be connected to at all, whereas loopback at least resolves.This makes
addresses.first()correct by construction, so the threeaddr()arms in the sender need no change.Ipv6Addr::is_unicast_link_localis still unstable, so the fe80::/10 test is done directly on the segments.Verification
7 new unit tests, including one asserting every rotation of the observed address set produces the same first address — the whole point of the change.
Confirmed against the live network: every discovered receiver now reports a routable IPv4 first, on every refresh.
fmt --checkclean,clippy --all-targets --all-features -D warningsclean,cargo test --all214 passed / 0 failed on macOS arm64.